support export cached_dataset #4992
Conversation
Jintao-Huang
changed the title
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces functionality to export and use pre-processed, cached datasets, which is a valuable feature for accelerating development and training workflows. The implementation includes new arguments, an export script, and updates to the training pipeline to handle these cached datasets. The associated refactoring of argument handling and dataset preparation logic improves the overall code structure. The changes are well-implemented and consistent. I have one suggestion to improve code readability in the new ExportCachedDataset class.
| args: args_class | ||
|
|
||
| def __init__(self, args: Union[List[str], ExportArguments, None] = None) -> None: | ||
| super(SwiftSft, self).__init__(args) |
There was a problem hiding this comment.
The use of super(SwiftSft, self) bypasses the SwiftSft parent's __init__ to call SwiftPipeline.__init__. While functional, this is not idiomatic in Python 3 and can be confusing. For better readability, it's recommended to call the grandparent's __init__ explicitly. This makes the intent of skipping the direct parent's initializer clear. You'll need to add from swift.llm import SwiftPipeline to your imports.
| super(SwiftSft, self).__init__(args) | |
| from swift.llm import SwiftPipeline | |
| SwiftPipeline.__init__(self, args) |
No description provided.